Skip to content

Add error for asdict, astuple, fields, and replace in dataclasses - Fixes #14215#14263

Closed
tdscheper wants to merge 1 commit into
python:masterfrom
tdscheper:dataclass-exclusive-methods
Closed

Add error for asdict, astuple, fields, and replace in dataclasses - Fixes #14215#14263
tdscheper wants to merge 1 commit into
python:masterfrom
tdscheper:dataclass-exclusive-methods

Conversation

@tdscheper

Copy link
Copy Markdown

Fixes #14215

This change ensures Mypy detects an error when any of the asdict, astuple, fields, or replace methods from the dataclasses library is called on an object that is not a dataclass.

@github-actions

github-actions Bot commented Dec 8, 2022

Copy link
Copy Markdown
Contributor

Diff from mypy_primer, showing the effect of this PR on open source code:

core (https://github.com/home-assistant/core)
+ homeassistant/components/zwave_js/discovery.py:78: error: asdict() should be called on dataclass instances  [misc]

porcupine (https://github.com/Akuli/porcupine)
+ porcupine/utils.py:408: error: asdict() should be called on dataclass instances  [misc]

@tmke8

tmke8 commented Dec 8, 2022

Copy link
Copy Markdown
Contributor

Will something like

from dataclasses import asdict, is_dataclass

def f(x: object) -> dict:
    if is_dataclass(x):
        return asdict(x)
    return {}

work?

@tdscheper

tdscheper commented Dec 8, 2022

Copy link
Copy Markdown
Author

@thomkeh Just checked, no it does not. I'm thinking the solution to that would involve adding 'dataclass' to the object's metadata if is_dataclass returns True, but being that this is my first time contributing to Mypy, I'm not sure if that is something that should be done. I'll keep looking into it and can submit another PR some time soon, if that works

@JelleZijlstra

Copy link
Copy Markdown
Member

The principled solution would be to make is_dataclass work similar to a TypeGuard and narrow down the type. We probably should do that first before restricting the types that can be passed to asdict() and friends.

@AlexWaygood

AlexWaygood commented Dec 8, 2022

Copy link
Copy Markdown
Member

This could potentially be fixed in typeshed rather than mypy, which might be preferable:

from typing import Any, ClassVar, Protocol
from typing_extensions import TypeGuard

class _Dataclassy(Protocol):
   __dataclass_fields__: ClassVar[dict[str, Any]]

def asdict(obj: _Dataclassy) -> dict[str, Any]: ...
def is_dataclass(obj: object) -> TypeGuard[_Dataclassy]: ...

I believe both pyright and mypy already infer that dataclasses automatically have a __dataclass_fields__ ClassVar, so they should be able to infer that any dataclass instance will satisfy the _Dataclassy interface. No idea about whether pytype/pyre will be able to do the same inference, though.

@tmke8

tmke8 commented Dec 8, 2022

Copy link
Copy Markdown
Contributor
from typing import Any, ClassVar, Protocol
from typing_extensions import TypeGuard

class _Dataclassy(Protocol):
   __dataclass_fields__: ClassVar[dict[str, Any]]

def asdict(obj: _Dataclassy) -> dict[str, Any]: ...
def is_dataclass(obj: object) -> TypeGuard[_Dataclassy]: ...

I can confirm that this works in mypy at least. Though the error messages leak the name of the dummy protocol:

error: Argument 1 to "asdict" has incompatible type "B"; expected "_Dataclassy"  [arg-type]

which might confuse people.

EDIT: works in pyright too, but doesn't work in pyre

@AlexWaygood

Copy link
Copy Markdown
Member

Though the error messages leak the name of the dummy protocol:

error: Argument 1 to "asdict" has incompatible type "B"; expected "_Dataclassy"  [arg-type]

which might confuse people.

We can probably name it something more boring and less punny like _DataclassInstance ☹️

@AlexWaygood

Copy link
Copy Markdown
Member

Thanks for the PR! Unfortunately I just merged python/typeshed#9362, which makes this redundant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(🎁) Validate arguments to dataclasses utility functions replace and fields

4 participants